Getting rid of build warnings #110
Conversation
|
CLA signature looks good 👍 |
|
Hey @mveritym, Thanks so much for this - very excited to see you've made tota11y the target of your first open source PR!! I'll give this a look today - but at a glance everything looks really great, this is going to be really valuable. |
package.json
Outdated
| @@ -32,7 +31,7 @@ | |||
| "scripts": { | |||
| "build": "npm run prod && npm run dev", | |||
| "prod": "webpack --config webpack.config.babel.js -p", | |||
| "dev": "webpack --config webpack.config.babel.js -d --devtool hidden --output-file=tota11y.js", | |||
| "dev": "webpack --config webpack.config.babel.js -d --devtool hidden --output=tota11y.js", | |||
There was a problem hiding this comment.
Looks like tota11y.js is no longer being generated because of this line. Is that possible?
| @@ -9,7 +9,6 @@ | |||
| "author": "Jordan Scales <[email protected]>", | |||
| "devDependencies": { | |||
| "accessibility-developer-tools": "2.9.0-rc.0", | |||
| "autoprefixer-loader": "^2.0.0", | |||
There was a problem hiding this comment.
Do we need to configure autoprefixing within postcss now? It seems some properties like align-items are not being prefixed. Can you confirm?
For reference, here's how .tota11y-plugin-switch is defined currently on master:
webpack.config.babel.js
Outdated
| @@ -58,6 +58,7 @@ module.exports = { | |||
| new webpack.ProvidePlugin({ | |||
| [options.jsxPragma]: path.join(__dirname, "utils", "element"), | |||
| }), | |||
| new webpack.optimize.UglifyJsPlugin({compress: {warnings: false}}) | |||
There was a problem hiding this comment.
Oh hmmm, I'm noticing that this is minifying tota11y.js too, but we want to keep that uncompressed.
I noticed this by running:
rm -rf build/
npm run build
and then viewing the contents of build/tota11y.js. Is there an easy way we can make this plugin only apply for the production case? (See prod in package.json's "scripts" field).
|
Apologies for the delayed review. I went on vacation and forgot to drop an update here that I was a bit behind schedule. |
|
@jdan no worries about the delay, hope you enjoyed your holiday! Fixed the pretty major stuff I missed, thanks for the review 🙂 |
webpack.config.babel.js
Outdated
| ], | ||
| postcss: [veryimportant], | ||
| postcss: [veryimportant, autoprefixer], |
There was a problem hiding this comment.
Looks like we'll need our browser config from above here. That is, {browsers:['> 1%']} removed from above on line 62 and passed into the autoprefixer plugin like so:
[veryimportant, autoprefixer({ browsers: ["> 1%"] })]
I think that will do the trick!
webpack.config.babel.js
Outdated
| @@ -58,6 +58,7 @@ module.exports = { | |||
| new webpack.ProvidePlugin({ | |||
| [options.jsxPragma]: path.join(__dirname, "utils", "element"), | |||
| }), | |||
| new webpack.optimize.UglifyJsPlugin({compress: {warnings: false}}) | |||
There was a problem hiding this comment.
Oh hmmm, I'm noticing that this is minifying tota11y.js too, but we want to keep that uncompressed.
I noticed this by running:
rm -rf build/
npm run build
and then viewing the contents of build/tota11y.js. Is there an easy way we can make this plugin only apply for the production case? (See prod in package.json's "scripts" field).
|
Hey @mveritym, Just checking in - I'm going to be making a couple tweaks on here to fix the little nits I pointed out. Feel free to check out the commits that appear at the bottom of this page and comment as you see fit! I just fixed the issue with the uglifyJS plugin usage (thanks to you for teaching me how we could include that in the babel config!) so that it conditionally added uglifyjs if we're in production mode. |
|
Hey @mveritym, I went ahead and squashed all your changes and landed them on master ✨ Just wanted to give a special thanks for tackling this - dependency upgrades are difficult yet super important tasks that really help to make projects like tota11y stand the test of time. I really appreciate all the hard work you put into the PR, and I am honored that you chose tota11y as the home of your first open source pull request! Feel free to reach out anytime if you're looking for another problem to tackle, or if you have any questions/comments on how we can make this project better for contributors. Take care, and thanks again! |
Fixes #90
Feedback welcome, this is my first open source PR ever 🎉